fix(requests): enforce the quota when editing a request - #3378
fix(requests): enforce the quota when editing a request#3378fallenbagel wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PUT request handler serializes updates, reloads requests inside the lock, and enforces movie and TV quotas during updates and reassignment. Tests cover quota limits, reassignment, unlimited quotas, declined requests, and ChangesRequest quota enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to The quota enforcement change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant Client
participant PUTHandler
participant requestLock
participant RequestRepository
Client->>PUTHandler: Submit request update
PUTHandler->>requestLock: Lock by request and target user
requestLock->>RequestRepository: Reload current request
RequestRepository-->>requestLock: Current request
requestLock->>PUTHandler: Validate quotas and persist update
PUTHandler-->>Client: Return updated request
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
455c4c4 to
62f6a45
Compare
fallenbagel
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the /request/:requestId PUT handler to enforce quota constraints correctly (including during reassignment) and to serialize quota-sensitive edits via a per-user lock, with accompanying test coverage for quota edge cases.
Changes:
- Wrap request edits in a user-keyed
requestLock.dispatch(...)to prevent concurrent quota/race issues during PUT updates. - Add quota enforcement for TV season edits (delta vs full charge on reassignment) and for movie reassignment when the target user is at quota.
- Refactor and expand request route tests with seed helpers and new quota-focused test cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
server/routes/request.ts |
Adds user-keyed locking and new quota/reassignment checks in the PUT request update flow. |
server/routes/request.test.ts |
Adds seed helpers and introduces quota-specific PUT tests for TV seasons and reassignment scenarios. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
62f6a45 to
ba51a06
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ba51a06 to
ff14bb2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ff14bb2 to
0624bb8
Compare
Editing a request never checked the requester's quota, so a user could add seasons past their limit on an existing request and an admin could move a request onto a user with no room for it. An edit is charged for every season getQuota did not already count against the owner, so reassignment and a request older than the quota window pay in full while an ordinary edit pays the difference. The handler serializes on the request id from the url so both parties to a reassignment queue behind each other, then takes the owner lock around the quota check and save. Fixes #633
0624bb8 to
7594577
Compare
Description
Editing a request never checked the requester's quota at all. A user could add seasons beyond their limit on a request that already existed, which is #633, and an admin could move a request onto a user with no room for it.
An edit is charged for every season
getQuotadid not already count against the owner. Reassignment pays in full because none of the request's seasons count towards the new owner yet, a declined request pays in full because declined requests are excluded from the count, and a request older than the quota window pays in full because the window filters on when the request was created. An ordinary edit inside the window pays only the difference. Requests created with the quota bypass keep it, and users whose quota is unlimited are unaffected.The handler serializes on the request id from the url so both parties to a reassignment queue behind each other, then takes the owner lock from the PR below around the quota check and save. The prefix on that first key is load bearing, since a request id and a user id would otherwise stringify to the same key on the same lock instance.
How Has This Been Tested?
Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit